Skip to content

Fix/tray single instance and prevent duplicate plan display - #1524

Closed
ArrogHie wants to merge 3 commits into
GCWing:mainfrom
ArrogHie:fix/tray-single-instance
Closed

Fix/tray single instance and prevent duplicate plan display#1524
ArrogHie wants to merge 3 commits into
GCWing:mainfrom
ArrogHie:fix/tray-single-instance

Conversation

@ArrogHie

Copy link
Copy Markdown
Contributor

fix(desktop): prevent duplicate tray icons when multiple workspace instances are running on Windows
fix(flow-chat): prevent duplicate plan display and ACP permission options

BitFun Dev and others added 3 commits July 10, 2026 14:41
…stances are running on Windows

Use a named kernel mutex (CreateMutexW) to detect whether another BitFun process is already running.  Only the primary instance creates the system tray icon; secondary instances skip tray creation but otherwise run normally.  The existing OnceLock guard inside setup_tray() is kept as an in-process safety net.

The mutex handle is held by PRIMARY_MUTEX (OnceLock<isize>) for the lifetime of the primary process so the kernel object is never destroyed prematurely.  Non-primary instances intentionally leak their duplicate handle — harmless; reclaimed on process exit.
…ions

- Use deterministic planFilePath-based IDs instead of random IDs in
  appendPlanDisplayItemsIfNeeded so that addModelRoundItem dedup works.
- Add handledPlanDisplayTurns guard to ensure plan display items are
  injected at most once per (sessionId, turnId).
- Make finalizePendingTurnCompletionNow return boolean so the fallback
  finalizeTurnCompletionState call in handleSessionStateChanged skips
  when the pending was already processed.
- Add content-based dedup in addModelRoundItem for CreatePlan items
  that share the same plan_file_path as a defensive layer.
- Add idempotency checks in handleConfirmationNeeded and
  applyAcpPermissionRequest to skip duplicate confirmation events.
- Add debug logs at all dedup and idempotency guard points.
@GCWing
GCWing requested a review from bobleer July 13, 2026 08:32
@bobleer
bobleer requested review from limityan and removed request for bobleer July 13, 2026 08:36

@limityan limityan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审核结论

建议 Request changes。这两个修复方向本身是合理的,但当前实现仍有几处会影响多工作区可用性、事件状态正确性和 CI 的阻断问题。下面按“问题—风险—推荐方案”说明。

1. 次实例隐藏后没有恢复入口

问题:Windows 次实例不会创建托盘,但 minimize_to_tray 和启动阶段的关闭逻辑仍会直接隐藏它的窗口。主实例托盘只持有主进程自己的 AppHandle,不能恢复其他进程的窗口。

风险:用户关闭任意次实例后,该工作区会继续在后台运行,却无法从托盘重新打开,只能通过任务管理器结束,形成“幽灵进程”。

推荐方案:先明确多实例窗口契约,建议二选一:

  • 唯一托盘通过 IPC 维护所有实例并能激活指定窗口;或
  • 次实例不允许 hide-to-tray,改为普通最小化、保留任务栏入口,或明确退出。

setup_tray 对次实例的 no-op 不应继续被调用方理解为“本进程已经具备托盘恢复能力”。

相关位置:src/apps/desktop/src/api/system_api.rs:445-467src/apps/desktop/src/tray.rs:175-182

2. 次实例泄漏 mutex 句柄会阻止主实例重新选举

问题:次实例调用 CreateMutexW 后故意不关闭句柄。Windows 命名对象要等最后一个句柄关闭后才会销毁,因此主实例退出后,只要次实例仍在运行,这个 mutex 就仍然存在。

风险:A 为主实例、B 为次实例时,退出 A 后再启动 C,C 仍会被判断为次实例。此时 B、C 都没有托盘,系统会长期停留在“没有主实例但 mutex 仍存在”的状态。

推荐方案

  • 次实例立即关闭重复句柄,主实例句柄使用 RAII 管理;
  • 明确主实例退出后的重新选举或托盘接管策略;
  • mutex 应使用当前 Windows 会话的命名空间,而不是 Global\,避免同一用户的 RDP/快速切换会话互相压制托盘。

相关位置:src/apps/desktop/src/single_instance.rs:24-57

3. ACP/工具确认去重在事件重放后仍会失效

问题:ACP 去重依赖卡片当前保存的 acpPermission.permissionId,普通工具确认则只检查当前是否为 pending_confirmation。工具完成、拒绝或用户确认后,这些状态会被清除或改变;如果重连后再次投递同一事件,旧请求会重新把终态工具改回待确认。

风险:本 PR 想修复的“重复权限选项”仍可能在重连、事件重放或延迟事件场景复现,并且可能让已经完成/拒绝的工具再次弹出确认。

推荐方案:使用独立于 UI 卡片状态的幂等键,例如 (sessionId, toolCallId, permissionId/eventId);已完成、已拒绝等终态不得被旧确认事件回退。该记录需要在 session 删除、ACP 断开或 epoch 切换时清理,并补充“确认后重放”和“完成后重放”测试。

相关位置:AcpPermissionToolCardModule.ts:53-74ToolEventModule.ts:592-613

4. 立即完成路径提前删除了部分恢复信息

问题finalizePendingTurnCompletionNow 在调用最终处理前删除 pendingTurnCompletions,但最终处理还需要从这里读取 partialRecoveryReason

风险:原本应标记为 interrupted 的部分恢复失败会被显示成普通 completed,用户看不到正确的错误提醒。

推荐方案:在最终处理完成前保留 pending 记录,或先把 partialRecoveryReason 显式传入最终处理函数,再统一清理。建议增加“backend idle 触发立即完成 + partial recovery”的回归测试。

相关位置:EventHandlerModule.ts:1133-1140EventHandlerModule.ts:1163-1175

5. Plan 去重集合没有生命周期清理

问题:每个 turn 在确认是否真的存在 plan 文件前,就被加入 handledPlanDisplayTurns;session 删除、workspace 重置和全局清理路径都没有清理该集合。

风险:长时间运行时集合会按历史 turn 数持续增长;如果 plan 工具事件较晚到达,第一次检查没有发现 plan,后续也可能被该标记永久跳过。

推荐方案:优先用稳定 item ID 和 store 中的现有内容完成幂等,不再维护第二套全局集合。如果必须保留集合,只在成功注入后记录,并在 cleanupSessionBuffersclearAllBuffers 和 manager destroy 中对称清理。

相关位置:EventHandlerModule.ts:2684-2695TextChunkModule.ts:302-370

6. 当前 CI 与跨平台测试仍未满足合并条件

问题:当前 Frontend Build 已失败,因为 setup_tray 签名变化后没有同步启动性能契约测试;Rust matrix 因前置依赖被全部跳过。此外,新增 mutex 单测没有限定 Windows,但非 Windows 实现每次都返回 true,第二次调用断言为 false,因此 Linux/macOS 后续会确定失败。

风险:当前只看到前端第一处失败,不能把被跳过的 Rust 检查视为通过;机械修改字符串断言后仍会遇到跨平台失败。

推荐方案

  • 更新契约测试时保留真正语义:没有可恢复托盘时不得隐藏窗口;
  • Windows mutex 测试使用 #[cfg(target_os = windows)],并为非 Windows 行为添加对应测试;
  • 增加真实 Windows 多进程测试或清晰的手工验证记录,覆盖 A/B 启动、隐藏 B、退出 A、启动 C、不同用户会话等场景。

合并前建议

  • 先变基到最新 main 后重新检查;当前分支相对实时 main 已落后较多提交。
  • 当前改动约 +202/-11,但包含托盘与 FlowChat 两个独立问题以及 3 个 commit,建议至少整理提交历史,并考虑拆分独立 PR,降低回归和复核成本。
  • 重新运行完整 frontend tests、三平台 Rust tests,并记录 Windows 多实例手工验证结果。

@ArrogHie ArrogHie closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants